JMXFetch bundled integrations disabled by default - #696
Conversation
8ca167d to
9e65b7c
Compare
There was a problem hiding this comment.
Does the defaultEnabled have to be boxed?
There was a problem hiding this comment.
This allows null to be passed in for no default.
There was a problem hiding this comment.
Actually, that doesn't really work, so I'll change it to primitive.
There was a problem hiding this comment.
Yeah, you cast it to primitive anyway. And besides caller should make up their mind about this default :)
There was a problem hiding this comment.
This looks odd. Did you mean to test system property and env variable separately? Otherwise it is unclear if they actually work independently. Also you may want to check that false value here works as expected.
There was a problem hiding this comment.
They are tested separately by virtue of using different values. I did add more test cases though.
mar-kolya
left a comment
There was a problem hiding this comment.
Couple of minor comments that should be addressed, otherwise LGTM
Enable each individually by setting `-Ddd.integration.<integration_name>.enabled=true`.
9e65b7c to
4414dc2
Compare
Enable each individually by setting
-Ddd.integration.<integration_name>.enabled=true.